Skip to content
This repository was archived by the owner on Apr 29, 2019. It is now read-only.

Add Travis CI for automated linting and testing#36

Merged
Gisonrg merged 6 commits into
MarkBind:masterfrom
nicholaschuayunzhi:121-travis-setup
Mar 17, 2018
Merged

Add Travis CI for automated linting and testing#36
Gisonrg merged 6 commits into
MarkBind:masterfrom
nicholaschuayunzhi:121-travis-setup

Conversation

@nicholaschuayunzhi

@nicholaschuayunzhi nicholaschuayunzhi commented Mar 11, 2018

Copy link
Copy Markdown
Contributor

Related MarkBind/markbind#121

This pr introduces a script that builds a site based on the recently merged test site #27.

How it works

  1. run ESLint
  2. build site with local markbind-cli and npm published markbind
  3. compare _site with expected site
    • the script ignores diffs which are different path separators for html files
  4. if there were any errors, the build fails

The ignore path separator logic was written by me. I made it very specific so its likely there will be more false positives than negatives.

NOTE: need to set up Travis to run on this repo.

Tested on my fork:
nicholaschuayunzhi#3

@acjh acjh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

Comment thread test/test_site/testUtil/diffHtml.js Outdated
const jsdiff = require('diff');

/**
* Checks if fragment ends with a an unclosed path

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a

@acjh

acjh commented Mar 13, 2018

Copy link
Copy Markdown
Contributor

Are we able to reliably get the line number where the diff occurs?

Comment thread package.json Outdated
"chokidar": "^1.6.1",
"clear": "^0.0.1",
"commander": "^2.9.0",
"diff": "^3.5.0",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is only used in testing, put it under devDependencies

Comment thread test/test_site/test.sh Outdated
@@ -0,0 +1,14 @@
#! /bin/bash

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need space here

Comment thread test/test_site/test.sh Outdated
@@ -0,0 +1,14 @@
#! /bin/bash

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add comments for bash script :) It is not easy to read

Comment thread test/test_site/testUtil/test.js Outdated
const expected = fs.readFileSync(resolvedExpectedFilePath, 'utf8');
const actual = fs.readFileSync(resolvedActualFilePath, 'utf8');
if (path.parse(resolvedActualFilePath).ext !== '.html') {
if (expected !== actual) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if we want to compare asset files...what if you have two binary files?

Maybe just compare html files for now?

@nicholaschuayunzhi

Copy link
Copy Markdown
Contributor Author

Are we able to reliably get the line number where the diff occurs?

I'm currently looking into it. There doesn't seem to be an easy way to do it as of yet.

@acjh

acjh commented Mar 16, 2018

Copy link
Copy Markdown
Contributor

@Gisonrg Shall we merge this first?

You need to "Activate" Travis via https://travis-ci.org/MarkBind/markbind-cli as the repository owner.

@Gisonrg

Gisonrg commented Mar 16, 2018

Copy link
Copy Markdown
Contributor

Yea we can merge this first to see how it works :) I'll set up the Travis tomorrow.

@acjh acjh added this to the v1.6.1 milestone Mar 16, 2018
@Gisonrg Gisonrg merged commit 9957c5e into MarkBind:master Mar 17, 2018
@rachx

rachx commented Mar 21, 2018

Copy link
Copy Markdown
Contributor

Remember to update the developer guide (e.g. testing locally)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants